Skip to content

[drogon] Fix building errors on Windows and Linux#13007

Merged
BillyONeal merged 5 commits intomicrosoft:masterfrom
an-tao:drogon
Aug 21, 2020
Merged

[drogon] Fix building errors on Windows and Linux#13007
BillyONeal merged 5 commits intomicrosoft:masterfrom
an-tao:drogon

Conversation

@an-tao
Copy link
Copy Markdown
Contributor

@an-tao an-tao commented Aug 19, 2020

Describe the pull request

  • What does your PR fix? Fixes #
  1. After the PR [libmariadb] Include bundled zlib and openssl #12669, the zlib library is separated from libmariadb, so we should link against it explicitly.(fixed in Fix zlib link error on Windows drogonframework/drogon#545)

  2. Fix the following error on Linux.

截屏2020-08-20 上午12 40 11

@BillyONeal
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013 NancyLi1013 self-assigned this Aug 20, 2020
Comment thread ports/drogon/portfile.cmake
@NancyLi1013 NancyLi1013 added category:port-update The issue is with a library, which is requesting update new revision category:port-bug The issue is with a library, which is something the port should already support requires:author-response labels Aug 20, 2020
@an-tao an-tao requested a review from NancyLi1013 August 20, 2020 04:56
@an-tao an-tao marked this pull request as draft August 20, 2020 05:12
@an-tao
Copy link
Copy Markdown
Contributor Author

an-tao commented Aug 20, 2020

Hi, @NancyLi1013, this PR works on Windows and MacOS, but there is a little problem on Linux, after installing drogon, the drogon_ctl command in tools folder doesn't work (the libpq.so.5 can't be found), but it works in the buildtrees folder.
截屏2020-08-20 下午3 39 41
Although I have a workaround for this by export LD_LIBRARY_PATH=/../vcpkg/installed/x64-linux/lib, is there a better way to resolve this?

@an-tao an-tao marked this pull request as ready for review August 21, 2020 03:00
@BillyONeal
Copy link
Copy Markdown
Member

Would you like this to be merged as is even if the problem above isn't solved yet (since this is a version update)

@an-tao
Copy link
Copy Markdown
Contributor Author

an-tao commented Aug 21, 2020

That's ok, please merge it. thanks.

@NancyLi1013
Copy link
Copy Markdown
Contributor

NancyLi1013 commented Aug 21, 2020

@an-tao

This problem also exists on my local. We might fix it in another PR later.

I also noticed that this port requires libuuid to build on Linux. But there is no libuuid in the dependency lists.
Could you please help confirm this?

CMake Error at cmake_modules/FindUUID.cmake:98 (message):
  Could not find UUID
Call Stack (most recent call first):
  /13007/vcpkg/scripts/buildsystems/vcpkg.cmake:455 (_find_package)
  CMakeLists.txt:96 (find_package)
CMake Error at cmake_modules/FindUUID.cmake:98 (message):
  Could not find UUID
Call Stack (most recent call first):
  /13007/vcpkg/scripts/buildsystems/vcpkg.cmake:455 (_find_package)
  CMakeLists.txt:96 (find_package)

Only after installing uuid-dev, I can build successfully on my local. Since libuuid has existed in vpkg now.
We suggest to use the port that provided in vcpkg instead of system libraries.

So we might need to update this too.

@an-tao
Copy link
Copy Markdown
Contributor Author

an-tao commented Aug 21, 2020

@an-tao

This problem also exists on my local. We might fix it in another PR later.

I also noticed that this port requires libuuid to build. But there is no libuuid in the dependency lists.
Could you please help confirm this?

Yes, libuuid is required on Linux and MacOS but isn't on Windows. How do I add this ?

@an-tao
Copy link
Copy Markdown
Contributor Author

an-tao commented Aug 21, 2020

After added libuuid, I got following errors on Windows:
批注 2020-08-21 160253
@NancyLi1013 How should I handle this? Thanks.

Comment thread ports/drogon/CONTROL Outdated
@JackBoosY
Copy link
Copy Markdown
Contributor

@an-tao Please try again locally.

@an-tao
Copy link
Copy Markdown
Contributor Author

an-tao commented Aug 21, 2020

@an-tao Please try again locally.

It works, Thanks so much. Does vcpkg support FreeBSD? the libuuid is also not required on FreeBSD.
And should we use (!window&!uwp) ?

@JackBoosY
Copy link
Copy Markdown
Contributor

vcpkg support freebsd, but not totally. And freebsd is not one of our official triplets.
Windows includes uwp, so there is no need to add it.

@NancyLi1013 NancyLi1013 added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Aug 21, 2020
@BillyONeal BillyONeal merged commit 7b28594 into microsoft:master Aug 21, 2020
@BillyONeal
Copy link
Copy Markdown
Member

Thanks for your contribution!

@an-tao an-tao deleted the drogon branch August 22, 2020 01:50
remz1337 pushed a commit to remz1337/vcpkg that referenced this pull request Aug 23, 2020
Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-bug The issue is with a library, which is something the port should already support category:port-update The issue is with a library, which is requesting update new revision info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants